fix: convert adaptive thinking to enabled for Bedrock models that don't support it#220
Closed
blinkagent[bot] wants to merge 3 commits into
Closed
fix: convert adaptive thinking to enabled for Bedrock models that don't support it#220blinkagent[bot] wants to merge 3 commits into
blinkagent[bot] wants to merge 3 commits into
Conversation
…'t support it
When Claude Code sends thinking.type: "adaptive" (the default for
Opus 4.6 / Sonnet 4.6), Bedrock models that predate adaptive thinking
support (e.g. Sonnet 4.5 in GovCloud) reject the request with a 400
error.
This adds thinking parameter transformation to
augmentRequestForBedrock(): if the target model does not support
adaptive thinking, it converts {"type": "adaptive"} to
{"type": "enabled", "budget_tokens": 10000}.
Models that support adaptive thinking (Opus 4.6, Sonnet 4.6) are
detected by model name and left unchanged.
Fixes #219
|
I have used blink to create this PR. I will take a look at the code and will test it out before I ask for a review. |
|
I had a first pass through the code and I believe hardcoding the |
The Anthropic API requires budget_tokens < max_tokens, and Bedrock enforces a minimum of 1024 for budget_tokens. A hardcoded fallback of 10000 could violate the first constraint if max_tokens is small. Now adaptiveFallbackBudgetTokens() computes budget_tokens as 80% of max_tokens, clamped to the Bedrock minimum of 1024. When max_tokens is absent or zero, the default of 10000 is used. When max_tokens is too small to accommodate the minimum, half of max_tokens is used.
adaptiveFallbackBudgetTokens now returns (int64, bool). When max_tokens is present but too small to accommodate Bedrock's minimum budget_tokens of 1024, it returns (0, false) instead of halving the value below the minimum. convertAdaptiveThinkingForBedrock checks the ok return and skips the conversion on false, logging a warning and leaving the payload unchanged. This surfaces a clear upstream error rather than sending a known-bad budget_tokens that would cause a different 400. Fixes the edge case introduced in 252e414 where max_tokens <= 1280 produced a budget_tokens below minBedrockBudgetTokens. Updates TestConvertAdaptiveThinkingForBedrock to assert the conversion is skipped rather than asserting the previously broken half-value. Adds TestAdaptiveFallbackBudgetTokens to directly cover all input classes of the helper, including the two false cases.
Collaborator
|
@uzair-coder07 do we still need this? |
Collaborator
|
@uzair-coder07 following up again |
|
@dannykopping For some reason, I didn't receive a notification the first time around. With Github having issues constantly, I am not surprised. Thanks for pinging me again. I don't think this is needed. I will go ahead and close it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When Claude Code sends
thinking.type: "adaptive"(the default for Opus 4.6 / Sonnet 4.6), Bedrock models that predate adaptive thinking support (e.g. Sonnet 4.5 in GovCloudus-gov-west-1) reject the request with a 400 error:Changes
intercept/messages/base.gobedrockModelSupportsAdaptiveThinking()to detect whether the target Bedrock model supports adaptive thinking (only Opus 4.6 and Sonnet 4.6 do)convertAdaptiveThinkingForBedrock()which converts{"type": "adaptive"}to{"type": "enabled", "budget_tokens": 10000}when the target model doesn't support itaugmentRequestForBedrock()to call the new conversion after the model name remapintercept/messages/base_test.goTestBedrockModelSupportsAdaptiveThinking— 11 cases covering all model name formatsTestConvertAdaptiveThinkingForBedrock— 6 cases: conversion for non-4.6 models, preservation for 4.6 models, no-op forenabled/disabled/absent thinkingFixes #219